Skip to content

Conversation

@Firestar99
Copy link
Member

@eddyb mind if we move that code to declutter call?

Comment on lines 23 to 57
// HACK(eddyb) Rust 2021 `panic!` always uses `format_args!`, even
// in the simple case that used to pass a `&str` constant, which
// would not remain reachable in the SPIR-V - but `format_args!` is
// more complex and neither immediate (`fmt::Arguments` is too big)
// nor simplified in MIR (e.g. promoted to a constant) in any way,
// so we have to try and remove the `fmt::Arguments::new` call here.
#[derive(Default)]
struct DecodedFormatArgs<'tcx> {
/// If fully constant, the `pieces: &'a [&'static str]` input
/// of `fmt::Arguments<'a>` (i.e. the strings between args).
const_pieces: Option<SmallVec<[String; 2]>>,

/// Original references for `fmt::Arguments<'a>` dynamic arguments,
/// i.e. the `&'a T` passed to `fmt::rt::Argument::<'a>::new_*`,
/// tracking the type `T` and `char` formatting specifier.
///
/// E.g. for `format_args!("{a} {b:x}")` they'll be:
/// * `&a` with `typeof a` and ' ',
/// * `&b` with `typeof b` and 'x'
ref_arg_ids_with_ty_and_spec: SmallVec<[(Word, Ty<'tcx>, char); 2]>,

/// If `fmt::Arguments::new_v1_formatted` was used, this holds
/// the length of the `&[fmt::rt::Placeholder]` slice, which
/// currently cannot be directly supported, and therefore even
/// if all of `ref_arg_ids_with_ty_and_spec` are printable,
/// a much jankier fallback still has to be used, as it it were:
///
/// `format!("a{{0}}b{{1}}c\n with {{…}} from: {}, {}", x, y)`
/// (w/ `const_pieces = ["a", "b", "c"]` & `ref_args = [&x, &y]`).
has_unknown_fmt_placeholder_to_args_mapping: Option<usize>,
}
struct FormatArgsNotRecognized(String);

// HACK(eddyb) this is basically a `try` block.
let mut try_decode_and_remove_format_args = || {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make more sense to make these types (and the closure becoming a method) the real interface here, and name the module format_args_decompiler (because that's what this cursedness really is).

In theory we could allow non-panic uses of it, like instead of debug_printf! we could have debug_print! (or even the Rust std dbg! macro), that uses format_args! combined with a function that we treat just like we do panic entry-points.

Copy link
Member Author

@Firestar99 Firestar99 Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, feel free to rereview. Not a huge fan how I handled the Result, but wrapping it in a new type makes the function super ugly.

@Firestar99 Firestar99 force-pushed the move_panic_entry_point branch 2 times, most recently from fa07e53 to e434f66 Compare October 16, 2025 09:27
@Firestar99 Firestar99 force-pushed the move_panic_entry_point branch from e434f66 to 177162b Compare October 22, 2025 11:04
@Firestar99
Copy link
Member Author

Firestar99 commented Oct 22, 2025

Added a few more things:

  • move printf (and sample_param_permutation) macro implementation to their own mods
  • move debug_printf* functions in spirv-std's lib.rs to a new mod debug_printf

@Firestar99 Firestar99 changed the title move codegen for panic entry point to a separate mod move format args decompiler and support infra to separate mods Oct 22, 2025
@Firestar99 Firestar99 changed the title move format args decompiler and support infra to separate mods Move format args decompiler and support infra to separate mods Oct 22, 2025
@Firestar99 Firestar99 force-pushed the move_panic_entry_point branch from 177162b to 7db75b6 Compare October 22, 2025 11:11
@Firestar99 Firestar99 added this pull request to the merge queue Oct 22, 2025
@Firestar99
Copy link
Member Author

Wouldn't be surprised if the merge failed on a conflict in tests/compiletests/ui/arch/debug_printf_type_checking.stderr, exactly what this PR tries to rectify (see previous comment)

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Oct 22, 2025
@Firestar99 Firestar99 force-pushed the move_panic_entry_point branch from 7db75b6 to 5a623d2 Compare October 22, 2025 13:08
@Firestar99 Firestar99 enabled auto-merge October 22, 2025 13:08
@Firestar99 Firestar99 added this pull request to the merge queue Oct 22, 2025
Merged via the queue into main with commit 67ccf2f Oct 22, 2025
13 checks passed
@Firestar99 Firestar99 deleted the move_panic_entry_point branch October 22, 2025 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants